Conversation
…ort regex divine-web formats report descriptions with markdown bold (**Event ID:**) and uses "Reported Pubkey" instead of "Author Pubkey". The parse-report regex only matched divine-mobile's plain-text format, so tickets from web reports were never linked to Nostr events. syncZendeskAfterAction silently skipped them with "No linked open ticket found."
Zendesk requires Assignee, Category, and Issue to be set before a ticket can transition to Solved. Our API call added the internal note but silently failed to solve because these fields were empty. Sets assignee to the API user, Category to trust___safety, and Issue to other_content_report as defaults. Reports with specific categories already have these fields set by auto-categorize triggers at creation.
Violation type regex used \s which matches newlines, causing it to greedily capture across lines in plain-text (mobile) format. Switched to [^\n] to stop at line boundaries. Also corrected comment about custom_fields overwrite behavior to reflect actual call-ordering invariant rather than claiming Zendesk-level field protection.
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
I agree with the overall direction here. The root-cause analysis in the PR description is clear, the fix is well scoped to the real production issue, and the regex changes look like the right kind of tolerance for the format drift we've seen across Zendesk/client-generated content.
I do want to request two follow-ups before merge, mostly to keep this aligned with the patterns already used in this repo and to reduce regression risk.
First, the new required Zendesk custom field IDs in the solve path should not be hardcoded in application code:
payload.ticket.custom_fields = [
{ id: 14559549220879, value: 'trust___safety' },
{ id: 14560383908879, value: 'other_content_report' },
];This repo already treats Zendesk field IDs as environment config in wrangler.*.toml plus Env typing for the existing webhook callback fields. These two IDs have the same operational risk:
- they can silently break if the Zendesk form is changed
- they assume staging/prod schemas stay identical
- they're harder to discover and update when embedded in the middle of a fix path
Please move these to env vars and thread them through Env, consistent with the existing ZENDESK_FIELD_ACTION_* pattern. If you want, something like ZENDESK_FIELD_CATEGORY, ZENDESK_FIELD_ISSUE, plus the static values in code would be enough.
Second, this would benefit from a small amount of test coverage. The bug here came from format variation, and the changed logic is exactly the kind of thing that is easy to regress later. I'm not asking for a large test harness, just a few targeted cases around the parse-report behavior and solve payload behavior:
- plain text
Event ID/Author Pubkey/Violation Type - markdown-bold variants like
**Event ID:** Reported Pubkeyas well asAuthor Pubkey- the solve path including
assignee_emailand the required custom fields whensolve === true
To be clear, I'm not concerned about the broadened violation-type regex itself. On review it looks reasonable for the stated use here, and I don't see a reason to block on that part.
Requesting changes for the config hardcoding and targeted tests. Once those are in, this looks good to merge.
…ests Addresses Liz's review on PR #46: 1. Moved hardcoded Category and Issue field IDs to ZENDESK_FIELD_CATEGORY and ZENDESK_FIELD_ISSUE env vars in all wrangler configs, matching the existing ZENDESK_FIELD_ACTION_* pattern. 2. Added targeted tests for parse-report regex (plain text, markdown bold, Reported Pubkey variant, multi-word violation types, auth rejection) and env var threading for auto-solve required fields.
|
Both items addressed in
88 tests passing, verified running locally. |
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
The remaining gap is closed. The PR now has a real behavioral test covering the solved-ticket path through → → , and it asserts the actual Zendesk payload includes , , and the required custom fields.
I also made the parse-report tests hermetic so they no longer hit Zendesk during the suite. With that in place, this is ready to merge.
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
The remaining gap is closed. The PR now has a real behavioral test covering the solved-ticket path through handlePublish -> syncZendeskAfterAction -> addZendeskInternalNote, and it asserts the actual Zendesk payload includes status: solved, assignee_email, and the required custom fields.
I also made the parse-report tests hermetic so they no longer hit Zendesk during the suite. With that in place, this is ready to merge.
|
Re-checked the current head ( The remaining blocker is closed: the PR now has real behavioral coverage for the solved-ticket path, including the outbound Zendesk payload ( I also re-ran the worker suite locally against this branch and got 88/88 passing. |
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
Re-checked the current head and I’m comfortable merging this.
Summary
Context
Aleysha reported ticket #926 was actioned in relay admin but didn't auto-solve in Zendesk. Investigation traced through the full chain:
syncZendeskAfterActionran,addZendeskInternalNoteadded the note withstatus: 'solved', but Zendesk silently ignored the status change because required fields (Assignee, Category, Issue) were empty.Also fixed the "Auto-categorize Other Content Reports" Zendesk trigger directly in admin (was checking
csamtag instead ofother, copy-paste from CSAM categorizer).Related: divine-relay-manager#41 tracks per-moderator identity through to Zendesk assignee when Keycast Teams is integrated.
Test plan